Skip to content

add importpath_aliases to go_proto_library#2608

Merged
jayconrod merged 3 commits intobazel-contrib:masterfrom
ConsultingMD:master
Aug 20, 2020
Merged

add importpath_aliases to go_proto_library#2608
jayconrod merged 3 commits intobazel-contrib:masterfrom
ConsultingMD:master

Conversation

@tian000
Copy link
Copy Markdown
Contributor

@tian000 tian000 commented Aug 17, 2020

What type of PR is this?
Bug fix

What does this PR do? Why is it needed?
It allows go_proto_library to accept importpath_aliases as an argument. It is needed because gazelle generates go_proto_library rules that have importpath_aliases as an argument.

Which issues(s) does this PR fix?
Fixes #2358

@jayconrod
Copy link
Copy Markdown
Collaborator

This PR seems incomplete? It doesn't actually implement importpath_aliases or test that it works.

@tian000
Copy link
Copy Markdown
Contributor Author

tian000 commented Aug 18, 2020

@jayconrod in Line 19 of tests/core/go_proto_library/protos_alias_test.go, I import the protos using the alias path, and then ensure that the generated types exist.

@tian000
Copy link
Copy Markdown
Contributor Author

tian000 commented Aug 18, 2020

Im pretty new to rules_go but i think it works out of the box because go_context grabs the attribute here:
https://github.com/ConsultingMD/rules_go/blob/3b5f0c950e0deec63cd77cdcec7765b5f41fefbd/go/private/context.bzl#L441

Copy link
Copy Markdown
Collaborator

@jayconrod jayconrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I'm sorry, you're totally right. I forgot how this worked and was thinking that adding this attribute would be a lot more complicated.

This PR looks good, just have two small comments. Thanks for fixing this.

Comment thread tests/core/go_proto_library/protos_alias_test.go Outdated
Comment thread tests/core/go_proto_library/BUILD.bazel
@tian000 tian000 requested a review from jayconrod August 19, 2020 23:12
@tian000
Copy link
Copy Markdown
Contributor Author

tian000 commented Aug 19, 2020

np! @jayconrod responded to your comments!

Copy link
Copy Markdown
Collaborator

@jayconrod jayconrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@jayconrod jayconrod merged commit 631e26b into bazel-contrib:master Aug 20, 2020
jayconrod pushed a commit to jayconrod/rules_go that referenced this pull request Aug 21, 2020
yushan26 pushed a commit to yushan26/rules_go that referenced this pull request Jun 16, 2025
…ntrib#2712)

The validating `py_test` generated by `compile_pip_requirements` chokes
when the source `requirements.txt` is stored read-only, such as when
managed by the Perforce Helix Core SCM. Though `dependency_resolver`
makes a temporary copy of this file, it does so w/ `shutil.copy` which
preserves the original read-only file mode. To address this, this commit
replaces `shutil.copy` with a `shutil.copyfileobj` such that the
temporary file is created w/ permissions according to the user's umask.

Resolves (bazel-contrib#2608).

---------

Co-authored-by: Ignas Anikevicius <240938+aignas@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

importpath_aliases not implemented for go_proto_library

3 participants